refactor: repository structure & naming convention (#325) - #333

Review — repository structure & naming convention refactor (deep, multi-dimension pass)

Reviewed across behavior-preservation, dangling references, convention consistency, docs, silent breakage, naming design, test integrity, and cruft — plus a manual lead pass. Verdict: mechanically clean and behavior-preserving. I ran the full gate locally on the head: go build ./... ✓, go vet ./... ✓, go test ./... (all 225 pkgs) exit 0 ✓ (golangci-lint surfaced only wsl deprecation warnings, no errors). Coverage is preserved (0 test deletions; the apparent elb↔elbv2 / gce / cloudmonitoring "swaps" all land correctly with matching package decls), no logic was smuggled into any rename hunk (the awsiam.*→bare-iam rewrite in the external iam_test package correctly binds to the AWS SDK iam, not the local package), no back-compat shims, no empty/orphaned dirs, and the convention itself is sound — no stutter, since types are sql.Mock/iam.Mock not sql.SqlServer. Nice work; this is a careful migration.

Findings are documentation + minor consistency — nothing blocking.

Should address — docs now contradict the convention this PR makes canonical

This PR promotes STRUCTURE.md to "source of truth" and adds a blocking check_structure.sh CI gate — so stale docs will send contributors to dead paths / into CI failures. These weren't edited here, but the renames staled them:

• docs/architecture.md (lines ~138, 158, 182–208, 238): the directory-tree diagrams still show awsiam/ azuremonitor/ azureiam/ azuredns/ azurelb/ azurecache/ azuresql/ gcpvpc/ gcpiam/ gcplb/ azureai/ azuresearch/.
• docs/services.md (coverage matrix rows ~13–26; SQL section ~1503/1665–1676): old provider names.
• docs/sdk-server.md:253: azuresql/ → sql/.
  Worth folding into this PR so the docs and the new gate agree on day one.

Minor consistency (introduced or left by this PR)

• Struct fields keep the prefix the package dropped: providers/azure/azure.go:147-148 — AzureAI *ai.Mock, AzureSearch *search.Mock, while every sibling is unprefixed (IAM, DNS, SQL, Cache, Monitor, LB). Rename to AI/Search.
• Primary filenames keep the old prefix in three Azure dirs: cache/azurecache.go, search/azuresearch.go, sql/azuresql.go (+azuresql_test.go) — whereas dns/dns.go, monitor/monitor.go, iam/iam.go, ai/ai.go dropped it. §4 wants the primary file = <service>.go; either rename or note the exception.
• machinelearning_more.go sits next to machine_learning.go (in providers/azure/ai/ and the services driver) — snake_case (§3) applied unevenly.

Docs precision (in the new STRUCTURE.md)

• §2.2 says provider/wire "keep the name that cloud's SDK uses," but the chosen leaves (cache, monitor, dns, sql) are not Azure's SDK names (rediscache, azuremonitor, cognitivesearch, …). Reword to "short capability name, no cloud prefix" — that's what the code actually does.
• §5's traffic_mirror/ sub-surface example is hypothetical and contradicts the real flat layout (providers/aws/vpc/traffic_mirror.go, server/aws/ec2/traffic_mirror.go) and §5.2's own "vpc stays flat" rule. Label it illustrative or use a real example.
• Two real exceptions the code makes but the doc doesn't call out: (a) AWS providers/aws/vpc ↔ server/aws/ec2 name split (pre-existing and defensible — AWS's SDK puts VPC under EC2 — but §2.2's "same name" rule reads as absolute); (b) services/azureai/services/azuresearch stay prefixed while providers/azure/ai/search dropped it (fine if treated as product-specific per §2.1, but say so).

Consider — the new CI gate has blind spots

scripts/check_structure.sh uses a single-level glob (for path in "$dir"/*/) with no recursion, so a cloud-prefixed dir one level deeper — exactly the promoted-subdir layout §4 now permits — passes the gate; and it doesn't scan services/ at all. Worth hardening so the enforcement actually covers what the convention allows.

One thing I checked and cleared

A diff against a stale base made it look like the PR also adds ~155 files + 3 deps (artifactregistry/eventarc/storageaccount). It doesn't — the authoritative diff is 263 renames + exactly 2 new files (STRUCTURE.md, check_structure.sh); those services are already in development. Clean rename scope.

Bottom line: behavior-preserving, green across build/vet/test, and a sound convention. I'd just sync architecture.md/services.md/sdk-server.md (the gate makes that matter now) and mop up the small field/filename inconsistencies. Merge call is yours.

Re-review — fixes in e8ecffc, bfb84ef, a98020c

Verified the delta in source (build ✓, vet ✓ on the changed trees). All ten original findings are resolved — and one of the fixes introduced a new CI regression that needs a one-line fix before merge.

Resolved ✓

Nice, thorough turnaround, and I appreciate that you took the elbv2/compute/monitoring correction all the way to real SDK names on both layers.

⚠️ New regression — the Structure CI job is now broken (fix before merge)

e8ecffc deleted scripts/check_structure.sh but the workflow still invokes it:

# .github/workflows/ci.yml — structure job
      - name: naming & layout convention (docs/STRUCTURE.md)
        run: bash scripts/check_structure.sh

The script exists nowhere in the tree (find . -name check_structure.sh → empty), and there is no inline check in ci.yml — so the step runs bash scripts/check_structure.sh → No such file or directory → exit 127 → the structure job fails on every run. The commit message says "inline the Structure check into the workflow," but the inline block didn't land; only the deletion did.

Fix: either (a) restore scripts/check_structure.sh (recursive version), or (b) actually inline it — replace that run: with the check body, e.g.:

        run: |
          fail=0
          for cloud in aws azure gcp; do
            for layer in providers server; do
              [ -d "$layer/$cloud" ] || continue
              while IFS= read -r d; do
                base=$(basename "$d")
                case "$base" in ${cloud}*) echo "cloud-prefixed dir: $d"; fail=1;; esac
              done < <(find "$layer/$cloud" -mindepth 1 -type d)
            done
          done
          exit $fail

(adjust to match the recursion/exclusions you intend). Everything else is good to go once this is sorted.

Re-review — 50d2d9c verified ✅

The Structure-job regression is fully fixed. ci.yml now carries the real inline check (no dangling scripts/ reference), and it's the hardened, recursive version — find "$layer/$cloud" -mindepth 1 -type d catches a redundant cloud prefix at any depth, so a promoted sub-surface subdir can't reintroduce one.

Verified beyond the diff:

• Ran the inline block against the tree → fail=0 (passes, no cloud-prefixed dirs remain).
• Negative control → the case "$cloud"?* pattern correctly matches a would-be azurefoo, so the gate actually enforces (not a green no-op).
• Live CI is all green: Structure ✅, Build & Vet ✅, Test ✅, Lint ✅, Format ✅, Tidy ✅, CodeQL / gosec / govulncheck / Dependency Review / Contrib ✅.

That closes every item from all three rounds — behavior-preserving rename, docs synced, field/filename consistency, STRUCTURE.md precision + documented exceptions, and a working, recursive Structure gate. Nothing outstanding on my end. LGTM — merge call is yours.